Skip to content

feat: transfer to spending from HW wallet#616

Merged
jvsena42 merged 37 commits into
masterfrom
feat/hw-transfer-to-spending
Jul 8, 2026
Merged

feat: transfer to spending from HW wallet#616
jvsena42 merged 37 commits into
masterfrom
feat/hw-transfer-to-spending

Conversation

@jvsena42

@jvsena42 jvsena42 commented Jul 6, 2026

Copy link
Copy Markdown
Member

Refs #589

This PR adds the watch-only Transfer to Spending flow for funding a Lightning channel from a paired Trezor, with the on-chain funding transaction signed on the device. It builds on the connect-hardware flow (#614) and is an iOS port of bitkit-android's transfer-to-spending flow (synonymdev/bitkit-android#1039).

Description

  • Adds the Amount → Sign With Your Device → Transaction Signed flow, entered from the Hardware Wallet detail screen's Transfer To Spending button, then handing off to the existing Funds in Transfer progress screen.
  • Sources the available amount and MAX from the device's native-segwit account balance, minus the LSP fee and a composed on-chain mining-fee reserve, while respecting the LSP receiving-cap limit.
  • Composes the funding transaction, signs it on the Trezor, broadcasts it, then records the actual mining fee, fee rate, and channel on the transfer activity.
  • Confirms the hardware-wallet transfer activity from watcher data without clobbering its existing transfer metadata.
  • Prevents the total balance from double-counting during the paid-order → pending-channel → ready-channel transition by resolving each transfer's channel via its funding transaction.
  • Shows the shared Spending intro before the first hardware transfer, then continues straight to the hardware amount flow on subsequent transfers.
  • Hardens the signing path: reconnects a known Bluetooth device before signing, clears a stale session on a signing timeout, and gives the Trezor Bridge signing call a longer read timeout so on-device confirmation doesn't time out at 30s.
  • Reuses the existing transfer machinery — spending-limit calculation, Blocktank order, channel watcher, and the Funds in Transfer screen — swapping only local signing for on-device signing.

Because iOS is Bluetooth-only in production, the Android USB/Bridge stale-session handling is limited to what the dev/E2E Bridge needs (the longer signing read timeout). The device-signing orchestration is extracted into a small HwFundingSigner collaborator so the transfer view model keeps only the coordination that reuses the shared machinery. v1 funds from the native-segwit account only; multi-address-type spend is out of scope.

QA Notes

OBS: driven on the Trezor Bridge emulator on regtest; please re-verify the full happy path and confirmation on a physical device.

Manual Tests

  • 1. Hardware Wallet detail → Transfer To Spending → Amount: shows TRANSFER TO SPENDING, AVAILABLE from the device balance, and the 25% / MAX / unit-toggle controls.
  • 2. First-ever hardware transfer → Spending intro shows, Continue advances to the hardware Amount flow; subsequent transfers skip the intro.
  • 3. Amount → 25% or MAX → Continue → Sign: a Blocktank order is created and Sign shows the fee grid with the Trezor visual behind the button.
  • 4. Sign → Open Trezor Connect → approve on device: composes, signs, broadcasts, advances to Transaction Signed, then auto-forwards to Funds in Transfer; spending increases and the hardware balance drops by the signed send.
  • 5. MAX with a low LSP receiving cap: AVAILABLE is capped by the receivable amount and the wallet spends only the LSP fee plus the composed mining fee.
  • 6. After the funding tx confirms: the transfer activity confirms from watcher data and shows the actual composed mining fee.
  • 7. regression: Total Balance during paid-order → pending-channel → ready-channel does not double count the transfer amount while the pending channel appears.
  • 8a. BLE reconnect: device unlocked → Open Trezor Connect: silently reconnects, signs, and broadcasts.
    • 8b. on failure → Open Trezor Connect: shows the Reconnect Hardware Device error.
  • 9. Bridge signing: a long device-confirmation pause does not time out before approval (previously failed at 30s).
  • 10. Amount above the limit: input resets to MAX and shows the max-amount error toast.
  • 11a. Sign → Learn More: opens the liquidity info screen.
    • 11b. Sign → Advanced → adjust receiving balance: returns to Sign with updated fees.

Automated Checks

  • Unit tests added: BitkitTests/HwFundingSignerTests.swift (9), BitkitTests/TransferViewModelHwTests.swift (5), BitkitTests/HwWalletManagerFundingTests.swift (4), and shared BitkitTests/HwTransferMocks.swift — cover the fee-reserve math (rate + both fallbacks), the reconnect/compose/sign/timeout orchestration and stale-session cleanup, the view model's error mapping and signing re-entrancy guard, and the native-segwit funding balance + funding-account resolution. 18/18 pass on the iPhone 16 simulator.
  • Build passes: xcodebuild -workspace Bitkit.xcodeproj/project.xcworkspace -scheme Bitkit -configuration Debug -destination 'platform=iOS Simulator,id=<iPhone 16>' ONLY_ACTIVE_ARCH=YES build (concrete simulator UDID + ONLY_ACTIVE_ARCH=YES because the Rust xcframeworks are arm64-only).
  • node scripts/validate-translations.js: 0 errors (run with glob resolved locally, since this checkout has no package.json).
  • SwiftFormat: clean on changed files.
  • No unit tests for the BalanceManager double-count fix or TransferService channel-resolution/activity marking: iOS's LightningService/BlocktankService/CoreService lack the protocol seams Android's repos have, so these are covered by the regtest E2E rather than isolated unit tests.
  • CI: standard build and test checks run by the PR bot.

Linked Issues/Tasks

Screenshot / Video

transfer-emulator.mov
close-channel.mov

@jvsena42 jvsena42 self-assigned this Jul 6, 2026
@jvsena42 jvsena42 added this to the 2.4.0 milestone Jul 6, 2026
@jvsena42 jvsena42 mentioned this pull request Jul 6, 2026
9 tasks
@jvsena42

jvsena42 commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

waiting for upstream branch merge

Base automatically changed from feat/hw-wallet-connect to master July 6, 2026 18:11
@jvsena42 jvsena42 marked this pull request as ready for review July 6, 2026 21:11
@piotr-iohk

This comment was marked as outdated.

@jvsena42 jvsena42 marked this pull request as draft July 7, 2026 11:05
auto-merge was automatically disabled July 7, 2026 11:05

Pull request was converted to draft

@jvsena42

jvsena42 commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

draft for implement fixes

@jvsena42 jvsena42 marked this pull request as ready for review July 7, 2026 11:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e293b4920e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread Bitkit/Views/Transfer/Hardware/SpendingAmountHw.swift Outdated
Comment thread Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift
Comment thread Bitkit/ViewModels/TransferViewModel.swift
Comment thread Bitkit/Managers/BalanceManager.swift
@jvsena42 jvsena42 marked this pull request as draft July 7, 2026 12:01
@jvsena42

jvsena42 commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

draft for check comments

@jvsena42 jvsena42 marked this pull request as ready for review July 7, 2026 16:12
Comment thread Bitkit/ViewModels/HwFundingSigner.swift

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d13e31089

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread Bitkit/Managers/TrezorManager.swift
Comment thread Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift Outdated
@jvsena42 jvsena42 requested a review from piotr-iohk July 7, 2026 16:44
@jvsena42

jvsena42 commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

code re-checked and video updated

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Manual QA — PR #616 retest (HW transfer to spending)

Retested on latest HEAD after follow-up fixes. Physical Trezor Safe 7, BLE, regtest (not Bridge emulator).

Attachments:
bitkit_logs_2026-07-08_08-12-00.zip


Verdict: request changes — one item left

Area Previous Retest
Happy path (sign → broadcast → in-transfer) ✅ Still good
Duplicate activities after transfer ✅ Fixed — single transfer row in Activity / home
BLE reconnect toast on sign screen optional follow-up ✅ INFO toast hardware__connect_error for known BLE device (Android parity)
Cancel on device Still shows generic Error toast

Everything from the earlier review looks addressed except cancel-on-device.


BLE reconnect INFO toast — ✅

When reconnect fails during HW transfer sign for a known BLE device, the app shows an INFO toast with hardware__connect_error instead of the ERROR reconnect copy used for USB — Android / #1062 parity. Confirmed on device and in logs (ble:4837BF24… reconnect failure @ 07:57:24).


Cancel on Trezor sign screen — still fix before merge

Repro: Transfer to Spending → Sign → Open Trezor Connect → cancel/decline on device.

Expected: silent no-op (Android / #1062); stay on sign screen, retry works.

Actual: generic Error toast still appears on device.

616-2.mp4

Note: Retry after cancel does work — I cancelled once, then signed successfully on the next attempt (c170d214… in logs). The blocker is UX only (spurious error toast), not flow breakage.

Logs (bitkit_foreground_2026-07-08_07-54-57.log, bitkit_foreground_2026-07-08_07-57-09.log):

[07:56:16] ERROR: App Error [BitkitCore.TrezorError.UserCancelled] - ServiceQueue: core
[07:56:16] PERF: signTxFromPsbt(...) took 12.09 seconds
...
[08:09:01] ERROR: App Error [BitkitCore.TrezorError.UserCancelled] - ServiceQueue: core
[08:09:01] PERF: signTxFromPsbt(...) took 22.51 seconds
[08:09:31] PERF: signTxFromPsbt(...) took 10.9 seconds   ← retry succeeded
[08:09:32] Created sent onchain activity for txid c170d214…

Why the partial fix doesn’t cover device: isTrezorUserCancellation() + TransferViewModel silent path were added, but ServiceQueue wraps core errors in AppError before they reach the view model. The helper only matches a direct TrezorError cast — on device the caught error is AppError(message: "App Error", debugMessage: "BitkitCore.TrezorError.UserCancelled"), so the check returns false → hwTransferError = .generic(...)SpendingHwSign toast.

Unit tests pass because mocks throw raw TrezorError.UserCancelled; TrezorCancellationTests even asserts wrapped AppError returns false, which matches production failure.

Recommendation:

  1. Extend isTrezorUserCancellation() to detect cancellation through AppError (e.g. parse debugMessage for TrezorError.UserCancelled / PinCancelled / PassphraseCancelled, or preserve/unpack the underlying error in AppError.init(error:) for TrezorError).
  2. Add a unit test that wraps TrezorError.UserCancelled in AppError(error:) and asserts isTrezorUserCancellation() is true — mirrors what ServiceQueue actually throws on device.
  3. Optionally add an integration-style TransferViewModelHwTests case where MockHwFunding.signError = AppError(error: TrezorError.UserCancelled) to guard the full path.

Recommendation: merge after cancel toast fix; duplicate-activity dedup, BLE reconnect INFO toast, and happy path look good on Safe 7 ✅

@jvsena42

jvsena42 commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Test after 3f799c2

deny-sign.mov

Thanks! I forgot I could test this flor from emulator

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Manual QA — PR #616 retest (HW transfer to spending)

Retested on latest HEAD after cancel fix (3f799c20). Physical Trezor Safe 7, BLE, regtest.

Attachments:
bitkit_logs_2026-07-08_11-40-37.zip


Verdict: approve

All items from earlier review pass on device.

Area Status
Happy path (sign → broadcast → in-transfer) ✅ Two transfers completed (a67804e3…, 35260708…)
Cancel on device ✅ Silent no-op; retry works
Duplicate activities ✅ Single transfer row in Activity / home
BLE reconnect INFO toast hardware__connect_error on known BLE reconnect miss

Cancel on device — fixed. Declined on Trezor twice; no generic Error toast. Logs show wrapped UserCancelled from ServiceQueue followed by the silent path:

[11:34:23] ERROR: App Error [BitkitCore.TrezorError.UserCancelled] - ServiceQueue: core
[11:34:23] INFO: Hardware transfer cancelled on device 'ble:DC67CFBD…'
[11:34:41] PERF: signTxFromPsbt(...) took 14.56 seconds   ← retry succeeded
[11:34:42] Created sent onchain activity for txid a67804e3…
...
[11:38:49] ERROR: App Error [BitkitCore.TrezorError.UserCancelled] - ServiceQueue: core
[11:38:49] INFO: Hardware transfer cancelled on device 'ble:DC67CFBD…'
[11:39:11] PERF: signTxFromPsbt(...) took 13.04 seconds   ← retry succeeded
[11:39:12] Created sent onchain activity for txid 35260708…

(bitkit_foreground_2026-07-08_11-30-58.log)

Contrast with pre-fix session in the same bundle (07-54-57.log @ 07:56:16): UserCancelled logged but no Hardware transfer cancelled on device line — matches the old generic-toast behaviour.

Recommendation: merge #616.

@jvsena42 jvsena42 enabled auto-merge July 8, 2026 11:44
@jvsena42 jvsena42 merged commit 01d5d5b into master Jul 8, 2026
11 checks passed
@jvsena42 jvsena42 deleted the feat/hw-transfer-to-spending branch July 8, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants